-
Notifications
You must be signed in to change notification settings - Fork 420
RI-7707 update the database analysis page #5189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RI-7707 update the database analysis page #5189
Conversation
…rybook stories - Migrate BarChart, DonutChart, and database-analysis components to styled-components - Add comprehensive Storybook stories for all major components - Integrate theme system and create reusable styled-components mixins - Fix circular dependencies and TypeScript errors - Remove SCSS modules and barrel exports causing issues
| /** | ||
| * Global styles for the application | ||
| * These styles are applied to the entire application using styled-components | ||
| */ | ||
| export const GlobalStyles = createGlobalStyle<{ theme: Theme }>` | ||
| :root { | ||
| // todo: replace with theme colors at some point | ||
| /* Type colors for Redis data types */ | ||
| --typeHashColor: #364cff; | ||
| --typeListColor: #008556; | ||
| --typeSetColor: #9c5c2b; | ||
| --typeZSetColor: #a00a6b; | ||
| --typeStringColor: #6a1dc3; | ||
| --typeReJSONColor: #3f4b5f; | ||
| --typeStreamColor: #6a741b; | ||
| --typeGraphColor: #14708d; | ||
| --typeTimeSeriesColor: #6e6e6e; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new patterns, affecting the whole app, should be ideally introduced in a separate PR with explanation.
Can't we use @ArtemHoruzhenko 's suggestion about extending theme with custom colors instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally these should not be custom, but I have not gotten a list of theme colors to use.
My suggestion for using css properties instead of even theme overrides is simply practical.
example:
--bg-color-neutral100: ${({ theme }) => theme.semantic.color.background.neutral100};
// ....
background-color: ${({ theme }) => theme.semantic.color.background.neutral100};
// vs
background-color: var(--bg-color-neutral100);If --var is defined at root level, it is accessible everywhere and is a lot shorter to type.
In this case, the colors defined are used in most if not all "key" related components.
In case of theme changes, this can be the single place where we need to apply the changes.
If we use the approach to override the theme, we should not override it but define our theme based on redis-ui theme. Why? Because in case that redis-ui decides to change the structure from theme.semantic.color.background.neutral100 to just theme.components.page.background, it will not be just matter of changing the override, but also everywhere the old path is used.
Of course this is just an opinion, the team can decide if we want to use any sort of override at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pd-redis I would prefer to use theme here so it will be aligned with everything else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it looks like we must extend existing theme with another semantic component (e.g. keyType.color = 'json' | 'hash' | ...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you @ArtemHoruzhenko on both comments. But the colors in the designs are from the update we're supposed to do next. This will for sure be migrated to the theme once we're on the same page as designers
cc. @KrumTy cc @DimoHG
redisinsight/ui/src/pages/database-analysis/components/top-namespace/TopNamespace.spec.tsx
Outdated
Show resolved
Hide resolved
redisinsight/ui/src/pages/database-analysis/components/top-keys/TopKeysTable.tsx
Show resolved
Hide resolved
redisinsight/ui/src/pages/database-analysis/DatabaseAnalysisPageView.tsx
Outdated
Show resolved
Hide resolved
redisinsight/ui/src/pages/database-analysis/components/header/styles.module.scss
Show resolved
Hide resolved
- Create DatabaseAnalysisFactory using Fishery and Faker for consistent test data generation - Refactor TopKeys.spec.tsx to use factory instead of plain mock objects - Ensures all DatabaseAnalysis properties are properly set in tests
Code Coverage - Frontend unit tests
Test suite run success5407 tests passing in 701 suites. Report generated by 🧪jest coverage report action from 295361d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall LGTM
What
This PR migrates the database analysis page components from SCSS modules to styled-components, adds comprehensive Storybook stories, and resolves circular dependency issues.
Key Changes:
1. Styled-components Migration
BarChartandDonutChartfrom SCSS to styled-components with theme integrationTopNamespace,TopKeys,TableLoader,SummaryPerData,ExpirationGroupsView,GroupBadge, andTableTextBtnto styled-componentstheme.semantic.color.*,theme.core.space.*)2. Storybook Stories
Added comprehensive interactive stories for:
TopNamespace- 11 stories covering loading, empty states, data views, extrapolation, and custom delimitersTopKeys- 11 stories covering loading, various key types, TTL values, big keys, and custom delimitersGroupBadge- 8 stories covering key types, command groups, compressed mode, delete functionalityExpirationGroupsView- Stories for bar chart TTL visualizationDatabaseAnalysisPageView- Full page component storiesBarChartandDonutChart- Enhanced existing stories3. Styled-components Infrastructure
globalStyles.ts) with CSS variables for Redis data types and command groupsstyles/mixins/styledComponents.ts:truncateText- Text overflow handlingbreakpoint- Responsive media queriesscrollbarStyles- Custom scrollbar stylinginsightsOpen- Insights panel responsive statesCellTextcomponent props overridable while maintaining defaults4. TypeScript & Testing Improvements
TopNamespace.spec.tsxby creatingcreateMockedDatabaseAnalysishelperTopKeysTable.spec.tsxby properly typing Key arraysTable→TopKeysTableandTopNamespacesTableTesting
Manual Testing
Visual Verification
analysis-demo.mov